Skip to content

Fix duplicate conflict dialog when moving files across volumes#3772

Open
kristofszeles wants to merge 1 commit into
linuxmint:masterfrom
kristofszeles:fix-move-conflict-dialog-cross-volume
Open

Fix duplicate conflict dialog when moving files across volumes#3772
kristofszeles wants to merge 1 commit into
linuxmint:masterfrom
kristofszeles:fix-move-conflict-dialog-cross-volume

Conversation

@kristofszeles
Copy link
Copy Markdown

Summary

When moving a file to a different volume (different filesystem) and a file with the same name already exists at the destination, the file-conflict dialog is shown twice instead of once. The new filename entered by the user in the first dialog is silently dropped; only the name re-typed in the second dialog actually takes effect.

The same operation works correctly when copying between volumes — only move is affected.

Reproduction steps

  1. Mount two different filesystems — for example /home (your user partition) and an external USB drive mounted at /run/media/<user>/USB.
  2. Create a file foo.txt in your home directory and another file with the same name foo.txt on the USB drive.
  3. In Nemo, cut ~/foo.txt (Ctrl+X) and paste it into the USB drive (Ctrl+V). Drag-and-drop between the two volumes triggers the bug as well.
  4. The file-conflict dialog appears. Expand Select a new name for the destination, type a name that does not already exist on the USB drive (e.g. foo-renamed.txt), and click Rename.

Expected: the file is moved as foo-renamed.txt and the dialog closes.

Actual: the conflict dialog reappears, still showing the original conflict against foo.txt. Re-typing the same foo-renamed.txt and clicking Rename the second time succeeds.

The bug is not reproducible with copy (Ctrl+C / Paste) — only with move (Ctrl+X / Paste, or drag-and-drop between volumes).

Root cause

The bug lives in move_file_prepare() in libnemo-private/nemo-file-operations.c. The function exists to filter move operations into two buckets:

  • Files that can be moved by an atomic rename(2) (same filesystem).
  • Files that need to fall back to copy + delete (different filesystem), collected into a MoveFileCopyFallback list and processed later by copy_move_file().

For each file it performs a trial call to g_file_move() with G_FILE_COPY_NO_FALLBACK_FOR_MOVE. The behavior of GIO for that call is:

  1. It checks whether the destination exists. If so, and OVERWRITE is not set, it returns G_IO_ERROR_EXISTS.
  2. It calls rename(2). On a cross-filesystem move, rename(2) fails with EXDEV, which GIO maps to G_IO_ERROR_NOT_SUPPORTED.

So when moving foo.txt to a volume that already contains foo.txt, the sequence in move_file_prepare() is:

  1. g_file_move() returns G_IO_ERROR_EXISTS.
  2. The conflict dialog is shown. The user picks Rename → foo-renamed.txt. The local dest GFile is replaced with one pointing at foo-renamed.txt. goto retry.
  3. g_file_move() is called again. This time the existence check passes (foo-renamed.txt does not exist at the destination), and rename(2) is attempted. Because the source and destination are on different filesystems, it fails with G_IO_ERROR_NOT_SUPPORTED.
  4. The WOULD_RECURSE / WOULD_MERGE / NOT_SUPPORTED branch is taken, which calls:
    fallback = move_copy_file_callback_new (src, overwrite, position);
    MoveFileCopyFallback only stores src, overwrite and position — the renamed dest is discarded.
  5. Later, move_files() consumes the fallback and calls copy_move_file(src, job->destination, …). copy_move_file() recomputes the destination from scratch:
    dest = get_target_file (src, dest_dir, *dest_fs_type, same_fs);
    This produces the original name (foo.txt), not the renamed one.
  6. g_file_copy() returns G_IO_ERROR_EXISTS again, and copy_move_file() shows the conflict dialog a second time.
  7. The user re-types foo-renamed.txt. This time the rename happens inside copy_move_file()'s own retry loop, which actually performs the copy, so the operation succeeds.

The copy path is unaffected because nemo_file_operations_copy() calls copy_files() → copy_move_file() directly. There is no preliminary trial that throws away the user's chosen filename.

Fix

The bug is, at its core, a missing piece of state on the fallback record: when the user picks a new name in the prepare phase, that choice has no slot to live in until copy_move_file() runs. The fix adds that slot and propagates the user's chosen destination through the fallback, so copy_move_file() reuses it instead of recomputing the original target.

This keeps the existing UX intact — the conflict dialog still appears during the prepare phase (before scan_sources() walks the source tree), so the user can cancel a large cross-volume move at the conflict prompt without paying the cost of the scan first.

Changes in libnemo-private/nemo-file-operations.c

  1. MoveFileCopyFallback gains a GFile *precomputed_dest field:

    typedef struct {
        GFile *file;
        /* If set, the copy+delete move phase must use this destination
         * (e.g. after the user picked a new name in move_file_prepare). */
        GFile *precomputed_dest;
        gboolean overwrite;
        gboolean has_position;
        GdkPoint position;
    } MoveFileCopyFallback;
  2. A custom destructor move_file_copy_fallback_free() releases the reference and frees the struct:

    static void
    move_file_copy_fallback_free (gpointer data)
    {
        MoveFileCopyFallback *fallback = data;
    
        g_clear_object (&fallback->precomputed_dest);
        g_free (fallback);
    }
  3. move_copy_file_callback_new() takes the precomputed destination and g_object_refs it (NULL is allowed and means "compute it normally"):

    fallback->precomputed_dest = precomputed_dest != NULL
        ? g_object_ref (precomputed_dest)
        : NULL;
  4. copy_move_file() gains a GFile *precomputed_dest parameter (forward declaration and definition both updated). When non-NULL, it is used directly instead of calling get_target_file_with_custom_name() / get_target_file():

    if (unique_names) {
        dest = get_unique_target_file (src, dest_dir, same_fs,
                                       *dest_fs_type, unique_name_nr++);
    } else if (precomputed_dest != NULL) {
        /* Set by the cross-filesystem move fallback when the user already
         * picked a new name during move_file_prepare. Reuse that exact
         * destination so the conflict dialog is not shown a second time. */
        dest = g_object_ref (precomputed_dest);
    } else if (copy_job->target_name != NULL) {
        dest = get_target_file_with_custom_name (src, dest_dir, *dest_fs_type, same_fs,
                                                 copy_job->target_name);
    } else {
        dest = get_target_file (src, dest_dir, *dest_fs_type, same_fs);
    }
  5. move_file_prepare() passes the (possibly user-renamed) dest into the fallback at the point where the cross-FS retry returns NOT_SUPPORTED:

    fallback = move_copy_file_callback_new (src, dest, overwrite, position);
  6. move_files() forwards fallback->precomputed_dest into copy_move_file(), completing the round-trip from the prepare phase to the transfer phase.

  7. The other call sites of copy_move_file()copy_files() and the recursive call inside copy_move_directory() — pass NULL for the new parameter and remain behaviorally unchanged.

  8. move_job() frees the fallback list with move_file_copy_fallback_free instead of plain g_free, so the new GFile reference is released along with the rest of the fallback record.

Upstream

I have filed a pull request for the exact same issue against GNOME's official Nautilus repository with a slightly different solution: https://gitlab.gnome.org/GNOME/nautilus/-/merge_requests/2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant